Merge branch '340852-alert-metric-image-upload' into 'master'

Add upload & update endpoints for alert metrics

See merge request gitlab-org/gitlab!80982

doc/api/alert_management_alerts.md

@@ -11,6 +11,35 @@ This is the documentation of Alert Management Alerts API.
 NOTE:
 This API is limited to metric images. For more API endpoints please refer to the [GraphQL API](graphql/reference/index.md#alertmanagementalert).
 
+## Upload metric image
+
+```plaintext
+POST /projects/:id/alert_management_alerts/:alert_iid/metric_images
+```
+
+| Attribute   | Type    | Required | Description                          |
+|-------------|---------|----------|--------------------------------------|
+| `id`        | integer/string | yes      | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. |
+| `alert_iid` | integer | yes      | The internal ID of a project's alert. |
+
+```shell
+curl --request POST --header "PRIVATE-TOKEN: <your_access_token>"  --form 'file=@/path/to/file.png' \
+--form 'url=http://example.com' --form 'url_text=Example website' "https://gitlab.example.com/api/v4/projects/5/alert_management_alerts/93/metric_images"
+```
+
+Example response:
+
+```json
+{
+    "id": 17,
+    "created_at": "2020-11-12T20:07:58.156Z",
+    "filename": "sample_2054",
+    "file_path": "/uploads/-/system/alert_metric_image/file/17/sample_2054.png",
+    "url": "example.com/metric",
+    "url_text": "An example metric"
+}
+```
+
 ## List metric images
 
 ```plaintext
@@ -48,3 +77,34 @@ Example response:
     }
 ]
 ```
+
+## Update metric image
+
+```plaintext
+PUT /projects/:id/alert_management_alerts/:alert_iid/metric_image/:image_id
+```
+
+| Attribute   | Type    | Required | Description                          |
+|-------------|---------|----------|--------------------------------------|
+| `id`        | integer/string | yes      | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. |
+| `alert_iid` | integer | yes      | The internal ID of a project's alert. |
+| `image_id` | integer | yes      | The ID of the image. |
+| `url` | string | no      | The URL to view more metrics information. |
+| `url_text` | string | no      | A description of the image or URL. |
+
+```shell
+curl --header "PRIVATE-TOKEN: <your_access_token>" --request PUT  --form 'url=http://example.com' --form 'url_text=Example website' "https://gitlab.example.com/api/v4/projects/5/alert_management_alerts/93/metric_images/1"
+```
+
+Example response:
+
+```json
+{
+    "id": 23,
+    "created_at": "2020-11-13T00:06:18.084Z",
+    "filename": "file.png",
+    "file_path": "/uploads/-/system/alert_metric_image/file/23/file.png",
+    "url": "http://example.com",
+    "url_text": "Example website"
+}
+```

ee/app/models/alert_management/metric_image.rb

@@ -7,6 +7,10 @@ module AlertManagement
 
     belongs_to :alert, class_name: 'AlertManagement::Alert', foreign_key: 'alert_id', inverse_of: :metric_images
 
+    def self.available_for?(project)
+      project&.feature_available?(:alert_metric_upload)
+    end
+
     private
 
     def local_path

ee/app/models/ee/alert_management/alert.rb

@@ -21,6 +21,10 @@ module EE
 
         ::Deployments::AutoRollbackWorker.perform_async(environment.id)
       end
+
+      def metric_images_available?
+        ::AlertManagement::MetricImage.available_for?(project)
+      end
     end
   end
 end

ee/app/policies/ee/alert_management/alert_policy.rb

@@ -12,6 +12,7 @@ module EE
 
         rule { can?(:update_alert_management_alert) }.policy do
           enable :upload_alert_management_metric_image
+          enable :update_alert_management_metric_image
           enable :destroy_alert_management_metric_image
         end
       end

ee/app/services/alert_management/metric_images/upload_service.rb

+# frozen_string_literal: true
+
+module AlertManagement
+  module MetricImages
+    class UploadService < BaseService
+      attr_reader :alert, :file, :url, :url_text, :metric
+
+      def initialize(alert, current_user, params = {})
+        super
+
+        @alert = alert
+        @file = params.fetch(:file)
+        @url = params.fetch(:url, nil)
+        @url_text = params.fetch(:url_text, nil)
+      end
+
+      def execute
+        return ServiceResponse.error(message: _("You are not authorized to upload metric images"), http_status: :forbidden) unless can_upload_metrics?
+
+        metric = AlertManagement::MetricImage.new(
+          alert: alert,
+          file: file,
+          url: url,
+          url_text: url_text
+        )
+
+        if metric.save
+          ServiceResponse.success(payload: { metric: metric, alert: alert })
+        else
+          ServiceResponse.error(message: metric.errors.full_messages.join(', '), http_status: :bad_request)
+        end
+      end
+
+      private
+
+      def can_upload_metrics?
+        alert.metric_images_available? && current_user&.can?(:upload_alert_management_metric_image, alert)
+      end
+    end
+  end
+end

ee/lib/api/alert_management_alerts.rb

@@ -11,6 +11,51 @@ module API
 
     resource :projects, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       namespace ':id/alert_management_alerts/:alert_iid/metric_images' do
+        post 'authorize' do
+          authorize!(:upload_alert_management_metric_image, find_project_alert(request.params[:alert_iid]))
+
+          require_gitlab_workhorse!
+          ::Gitlab::Workhorse.verify_api_request!(request.headers)
+          status 200
+          content_type ::Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
+
+          params = {
+            has_length: false,
+            maximum_size: ::AlertManagement::MetricImage::MAX_FILE_SIZE.to_i
+          }
+
+          ::MetricImageUploader.workhorse_authorize(**params)
+        end
+
+        desc 'Upload a metric image for an alert' do
+          success Entities::MetricImage
+        end
+        params do
+          requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The image file to be uploaded'
+          optional :url, type: String, desc: 'The url to view more metric info'
+          optional :url_text, type: String, desc: 'A description of the image or URL'
+        end
+        post do
+          require_gitlab_workhorse!
+          bad_request!('File is too large') if max_file_size_exceeded?
+
+          alert = find_project_alert(params[:alert_iid])
+
+          authorize!(:upload_alert_management_metric_image, alert)
+
+          upload = ::AlertManagement::MetricImages::UploadService.new(
+            alert,
+            current_user,
+            params.slice(:file, :url, :url_text)
+          ).execute
+
+          if upload.success?
+            present upload.payload[:metric], with: Entities::MetricImage, current_user: current_user, project: user_project
+          else
+            render_api_error!(upload.message, upload.http_status)
+          end
+        end
+
         desc 'Metric Images for alert'
         get do
           alert = find_project_alert(params[:alert_iid])
@@ -21,6 +66,32 @@ module API
             render_api_error!('Alert not found', 404)
           end
         end
+
+        desc 'Update a metric image for an alert' do
+          success Entities::MetricImage
+        end
+        params do
+          requires :metric_image_id, type: Integer, desc: 'The ID of metric image'
+          optional :url, type: String, desc: 'The url to view more metric info'
+          optional :url_text, type: String, desc: 'A description of the image or URL'
+        end
+        put ':metric_image_id' do
+          alert = find_project_alert(params[:alert_iid])
+
+          authorize!(:update_alert_management_metric_image, alert)
+
+          render_api_error!('Feature not available', 403) unless alert.metric_images_available?
+
+          metric_image = alert.metric_images.find_by_id(params[:metric_image_id])
+
+          render_api_error!('Metric image not found', 404) unless metric_image
+
+          if metric_image&.update(params.slice(:url, :url_text))
+            present metric_image, with: Entities::MetricImage, current_user: current_user, project: user_project
+          else
+            render_api_error!('Metric image could not be updated', 422)
+          end
+        end
       end
     end
 
@@ -30,6 +101,10 @@ module API
 
         ::AlertManagement::AlertsFinder.new(current_user, project, { iid: [iid] }).execute.first
       end
+
+      def max_file_size_exceeded?
+        params[:file].size > ::AlertManagement::MetricImage::MAX_FILE_SIZE
+      end
     end
   end
 end

ee/spec/models/alert_management/metric_image_spec.rb

@@ -15,4 +15,26 @@ RSpec.describe AlertManagement::MetricImage do
     it { is_expected.to validate_length_of(:url).is_at_most(255) }
     it { is_expected.to validate_length_of(:url_text).is_at_most(128) }
   end
+
+  describe '.available_for?' do
+    subject { described_class.available_for?(issue.project) }
+
+    before do
+      stub_licensed_features(alert_metric_upload: true)
+    end
+
+    let_it_be_with_refind(:issue) { create(:issue) }
+
+    context 'license enabled' do
+      it { is_expected.to eq(true) }
+    end
+
+    context 'license disabled' do
+      before do
+        stub_licensed_features(alert_metric_upload: false)
+      end
+
+      it { is_expected.to eq(false) }
+    end
+  end
 end

ee/spec/requests/api/alert_management_alerts_spec.rb

@@ -11,6 +11,192 @@ RSpec.describe API::AlertManagementAlerts do
   let_it_be(:user) { create(:user) }
   let_it_be(:alert) { create(:alert_management_alert, project: project) }
 
+  describe 'PUT /projects/:id/alert_management_alerts/:alert_iid/metric_images/authorize' do
+    include_context 'workhorse headers'
+
+    before do
+      project.add_developer(user)
+    end
+
+    subject { post api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/authorize", user), headers: workhorse_headers }
+
+    it 'authorizes uploading with workhorse header' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+    end
+
+    it 'rejects requests that bypassed gitlab-workhorse' do
+      workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER)
+
+      subject
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
+    context 'when using remote storage' do
+      context 'when direct upload is enabled' do
+        before do
+          stub_uploads_object_storage(MetricImageUploader, enabled: true, direct_upload: true)
+        end
+
+        it 'responds with status 200, location of file remote store and object details' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          expect(json_response).not_to have_key('TempPath')
+          expect(json_response['RemoteObject']).to have_key('ID')
+          expect(json_response['RemoteObject']).to have_key('GetURL')
+          expect(json_response['RemoteObject']).to have_key('StoreURL')
+          expect(json_response['RemoteObject']).to have_key('DeleteURL')
+          expect(json_response['RemoteObject']).to have_key('MultipartUpload')
+        end
+      end
+
+      context 'when direct upload is disabled' do
+        before do
+          stub_uploads_object_storage(MetricImageUploader, enabled: true, direct_upload: false)
+        end
+
+        it 'handles as a local file' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response.media_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+          expect(json_response['TempPath']).to eq(MetricImageUploader.workhorse_local_upload_path)
+          expect(json_response['RemoteObject']).to be_nil
+        end
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/alert_management_alerts/:alert_iid/metric_images' do
+    include WorkhorseHelpers
+    using RSpec::Parameterized::TableSyntax
+
+    include_context 'workhorse headers'
+
+    let(:file) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') }
+    let(:file_name) { 'rails_sample.jpg' }
+    let(:url) { 'http://gitlab.com' }
+    let(:url_text) { 'GitLab' }
+
+    let(:params) { { url: url, url_text: url_text } }
+
+    subject do
+      workhorse_finalize(
+        api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images", user),
+        method: :post,
+        file_key: :file,
+        params: params.merge(file: file),
+        headers: workhorse_headers,
+        send_rewritten_field: true
+      )
+    end
+
+    shared_examples 'can_upload_metric_image' do
+      it 'creates a new metric image' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:created)
+        expect(json_response['filename']).to eq(file_name)
+        expect(json_response['url']).to eq(url)
+        expect(json_response['url_text']).to eq(url_text)
+        expect(json_response['file_path']).to match(%r{/uploads/-/system/alert_management_metric_image/file/\d+/#{file_name}})
+        expect(json_response['created_at']).not_to be_nil
+        expect(json_response['id']).not_to be_nil
+      end
+    end
+
+    shared_examples 'unauthorized_upload' do
+      it 'disallows the upload' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(json_response['message']).to eq('403 Forbidden')
+      end
+    end
+
+    where(:user_role, :expected_status) do
+      :guest     | :unauthorized_upload
+      :reporter  | :unauthorized_upload
+      :developer | :can_upload_metric_image
+    end
+
+    with_them do
+      before do
+        # Local storage
+        stub_uploads_object_storage(MetricImageUploader, enabled: false)
+        allow_next_instance_of(MetricImageUploader) do |uploader|
+          allow(uploader).to receive(:file_storage?).and_return(true)
+        end
+
+        stub_licensed_features(alert_metric_upload: true)
+        project.send("add_#{user_role}", user)
+      end
+
+      it_behaves_like "#{params[:expected_status]}"
+    end
+
+    context 'file size too large' do
+      before do
+        stub_licensed_features(alert_metric_upload: true)
+        allow_next_instance_of(UploadedFile) do |upload_file|
+          allow(upload_file).to receive(:size).and_return(AlertManagement::MetricImage::MAX_FILE_SIZE + 1)
+        end
+      end
+
+      it 'returns an error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(response.body).to match(/File is too large/)
+      end
+    end
+
+    context 'error when saving' do
+      before do
+        project.add_developer(user)
+
+        allow_next_instance_of(::AlertManagement::MetricImages::UploadService) do |service|
+          error = double(success?: false, message: 'some error', http_status: :bad_request)
+          allow(service).to receive(:execute).and_return(error)
+        end
+      end
+
+      it 'returns an error' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(response.body).to match(/some error/)
+      end
+    end
+
+    context 'object storage enabled' do
+      before do
+        # Object storage
+        stub_licensed_features(alert_metric_upload: true)
+        stub_uploads_object_storage(MetricImageUploader)
+
+        allow_next_instance_of(MetricImageUploader) do |uploader|
+          allow(uploader).to receive(:file_storage?).and_return(true)
+        end
+        project.add_developer(user)
+      end
+
+      it_behaves_like 'can_upload_metric_image'
+
+      it 'uploads to remote storage' do
+        subject
+
+        last_upload = AlertManagement::MetricImage.last.uploads.last
+        expect(last_upload.store).to eq(::ObjectStorage::Store::REMOTE)
+      end
+    end
+  end
+
   describe 'GET /projects/:id/alert_management_alerts/:alert_iid/metric_images' do
     using RSpec::Parameterized::TableSyntax
 
@@ -62,4 +248,97 @@ RSpec.describe API::AlertManagementAlerts do
       it_behaves_like "#{params[:expected_status]}"
     end
   end
+
+  describe 'PUT /projects/:id/alert_management_alerts/:alert_iid/metric_images/:metric_image_id' do
+    using RSpec::Parameterized::TableSyntax
+
+    let!(:image) { create(:alert_metric_image, alert: alert) }
+    let(:params) { { url: 'http://test.example.com', url_text: 'Example website 123' } }
+
+    subject { put api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{image.id}", user), params: params }
+
+    shared_examples 'can_update_metric_image' do
+      it 'can update the metric images' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:success)
+        expect(json_response['url']).to eq(params[:url])
+        expect(json_response['url_text']).to eq(params[:url_text])
+      end
+    end
+
+    shared_examples 'unauthorized_update' do
+      it 'cannot update the metric image' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+        expect(image.reload).to eq(image)
+      end
+    end
+
+    where(:user_role, :public_project, :expected_status) do
+      :not_member | false | :unauthorized_update
+      :not_member | true  | :unauthorized_update
+      :guest      | false | :unauthorized_update
+      :reporter   | false | :unauthorized_update
+      :developer  | false | :can_update_metric_image
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(alert_metric_upload: true)
+        project.send("add_#{user_role}", user) unless user_role == :not_member
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public_project
+      end
+
+      it_behaves_like "#{params[:expected_status]}"
+    end
+
+    context 'user has access' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'feature is enabled' do
+        before do
+          stub_licensed_features(alert_metric_upload: true)
+        end
+
+        context 'metric image not found' do
+          subject { put api("/projects/#{project.id}/alert_management_alerts/#{alert.iid}/metric_images/#{non_existing_record_id}", user) }
+
+          it 'returns an error' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+            expect(json_response['message']).to eq('Metric image not found')
+          end
+        end
+
+        context 'metric image cannot be updated' do
+          let(:params) { { url_text: 'something_long' * 100 } }
+
+          it 'returns an error' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:unprocessable_entity)
+            expect(json_response['message']).to eq('Metric image could not be updated')
+          end
+        end
+      end
+
+      context 'feature not enabled' do
+        before do
+          stub_licensed_features(alert_metric_upload: false)
+        end
+
+        it 'returns an error' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('Feature not available')
+        end
+      end
+    end
+  end
 end

ee/spec/services/alert_management/metric_images/upload_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AlertManagement::MetricImages::UploadService do
+  subject(:service) { described_class.new(alert, current_user, params) }
+
+  let_it_be_with_refind(:project) { create(:project) }
+  let_it_be_with_refind(:alert) { create(:alert_management_alert, project: project) }
+  let_it_be_with_refind(:current_user) { create(:user) }
+
+  let(:params) do
+    {
+      file: fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg'),
+      url: 'https://www.gitlab.com'
+    }
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    shared_examples 'uploads the metric' do
+      it 'uploads the metric and returns a success' do
+        expect { subject }.to change(AlertManagement::MetricImage, :count).by(1)
+        expect(subject.success?).to eq(true)
+        expect(subject.payload).to match({ metric: instance_of(AlertManagement::MetricImage), alert: alert })
+      end
+    end
+
+    shared_examples 'no metric saved, an error given' do |message|
+      it 'returns an error and does not upload', :aggregate_failures do
+        expect(subject.success?).to eq(false)
+        expect(subject.message).to match(a_string_matching(message))
+        expect(AlertManagement::MetricImage.count).to eq(0)
+      end
+    end
+
+    context 'user does not have permissions' do
+      it_behaves_like 'no metric saved, an error given', 'You are not authorized to upload metric images'
+    end
+
+    context 'user has permissions' do
+      before_all do
+        project.add_developer(current_user)
+      end
+
+      it_behaves_like 'no metric saved, an error given', 'You are not authorized to upload metric images'
+
+      context 'with license' do
+        before do
+          stub_licensed_features(alert_metric_upload: true)
+        end
+
+        it_behaves_like 'uploads the metric'
+
+        context 'no url given' do
+          let(:params) do
+            {
+              file: fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg')
+            }
+          end
+
+          it_behaves_like 'uploads the metric'
+        end
+
+        context 'record invalid' do
+          let(:params) do
+            {
+              file: fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain'),
+              url: 'https://www.gitlab.com'
+            }
+          end
+
+          it_behaves_like 'no metric saved, an error given', /File does not have a supported extension. Only png, jpg, jpeg, gif, bmp, tiff, ico, and webp are supported/
+        end
+
+        context 'user is guest' do
+          before_all do
+            project.add_guest(current_user)
+          end
+
+          it_behaves_like 'no metric saved, an error given', 'You are not authorized to upload metric images'
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -41693,6 +41693,9 @@ msgstr ""
 msgid "You are not authorized to update this scanner profile"
 msgstr ""
 
+msgid "You are not authorized to upload metric images"
+msgstr ""
+
 msgid "You are now impersonating %{username}"
 msgstr ""
 

workhorse/internal/upstream/routes.go

@@ -318,9 +318,12 @@ func configureRoutes(u *upstream) {
 		// Group Import via UI upload acceleration
 		u.route("POST", importPattern+`gitlab_group`, upload.Multipart(api, signingProxy, preparers.uploads)),
 
-		// Metric image upload
+		// Issuable Metric image upload
 		u.route("POST", apiProjectPattern+`issues/[0-9]+/metric_images\z`, upload.Multipart(api, signingProxy, preparers.uploads)),
 
+		// Alert Metric image upload
+		u.route("POST", apiProjectPattern+`alert_management_alerts/[0-9]+/metric_images\z`, upload.Multipart(api, signingProxy, preparers.uploads)),
+
 		// Requirements Import via UI upload acceleration
 		u.route("POST", projectPattern+`requirements_management/requirements/import_csv`, upload.Multipart(api, signingProxy, preparers.uploads)),
 

workhorse/upload_test.go

@@ -141,6 +141,7 @@ func TestAcceleratedUpload(t *testing.T) {
 		{"POST", `/api/v4/projects/group%2Fsubgroup%2Fproject/packages/pypi`, true},
 		{"POST", `/api/v4/projects/9001/issues/30/metric_images`, true},
 		{"POST", `/api/v4/projects/group%2Fproject/issues/30/metric_images`, true},
+		{"POST", `/api/v4/projects/9001/alert_management_alerts/30/metric_images`, true},
 		{"POST", `/api/v4/projects/group%2Fsubgroup%2Fproject/issues/30/metric_images`, true},
 		{"POST", `/my/project/-/requirements_management/requirements/import_csv`, true},
 		{"POST", `/my/project/-/requirements_management/requirements/import_csv/`, true},