Unpublish details for confidential issues

* Implement deleting objects for S3 client
* Implement unpublishing service
* Rename publish to process
* Allow finding a confidential issue for status page

ee/app/finders/status_page/incidents_finder.rb

@@ -10,10 +10,10 @@
 #
 #     finder = StatusPage::IncidentsFinder.new(project_id: project_id)
 #
-#     # A single issue
-#     issue, user_notes = finder.find_by_id(issue_id)
+#     # A single issue (including confidential issue)
+#     issue = finder.find_by_id(issue_id)
 #
-#     # Most recent 20 issues
+#     # Most recent 20 non-confidential issues
 #     issues = finder.all
 #
 module StatusPage
@@ -25,23 +25,22 @@ module StatusPage
     end
 
     def find_by_id(issue_id)
-      execute.find_by_id(issue_id)
+      execute(include_confidential: true)
+        .find_by_id(issue_id)
     end
 
     def all
-      @sorted = true
-
-      execute
+      execute(sorted: true)
         .limit(MAX_LIMIT) # rubocop: disable CodeReuse/ActiveRecord
     end
 
     private
 
-    attr_reader :project_id, :with_user_notes, :sorted
+    attr_reader :project_id
 
-    def execute
+    def execute(sorted: false, include_confidential: false)
       issues = init_collection
-      issues = public_only(issues)
+      issues = public_only(issues) unless include_confidential
       issues = by_project(issues)
       issues = reverse_chronological(issues) if sorted
       issues

ee/app/services/status_page/publish_base_service.rb

@@ -12,14 +12,14 @@ module StatusPage
       return error_feature_not_available unless feature_available?
       return error_no_storage_client unless storage_client
 
-      publish(*args)
+      process(*args)
     end
 
     private
 
     attr_reader :project
 
-    def publish(*args)
+    def process(*args)
       raise NotImplementedError
     end
 
@@ -53,6 +53,12 @@ module StatusPage
       success(object_key: key)
     end
 
+    def delete(key)
+      storage_client.delete_object(key)
+
+      success(object_key: key)
+    end
+
     def limit_exceeded?(json)
       !Gitlab::Utils::DeepSize
         .new(json, max_size: Storage::JSON_MAX_SIZE)

ee/app/services/status_page/publish_details_service.rb

@@ -10,7 +10,7 @@ module StatusPage
   class PublishDetailsService < PublishBaseService
     private
 
-    def publish(issue, user_notes)
+    def process(issue, user_notes)
       json = serialize(issue, user_notes)
       key = object_key(json)
       return error('Missing object key') unless key

ee/app/services/status_page/publish_incident_service.rb

@@ -9,6 +9,7 @@ module StatusPage
   #
   # This services calls:
   # * StatusPage::PublishDetailsService
+  # * StatusPage::UnpublishDetailsService
   # * StatusPage::PublishListService
   class PublishIncidentService
     include Gitlab::Utils::StrongMemoize
@@ -23,22 +24,34 @@ module StatusPage
       return error_permission_denied unless can_publish?
       return error_issue_not_found unless issue
 
-      response = publish_details
+      response = process_details
       return response if response.error?
 
-      publish_list
+      process_list
     end
 
     private
 
     attr_reader :user, :project, :issue_id
 
+    def process_details
+      if issue.confidential?
+        unpublish_details
+      else
+        publish_details
+      end
+    end
+
+    def process_list
+      PublishListService.new(project: project).execute(issues)
+    end
+
     def publish_details
       PublishDetailsService.new(project: project).execute(issue, user_notes)
     end
 
-    def publish_list
-      PublishListService.new(project: project).execute(issues)
+    def unpublish_details
+      UnpublishDetailsService.new(project: project).execute(issue)
     end
 
     def issue

ee/app/services/status_page/publish_list_service.rb

@@ -10,7 +10,7 @@ module StatusPage
   class PublishListService < PublishBaseService
     private
 
-    def publish(issues)
+    def process(issues)
       json = serialize(issues)
 
       upload(object_key, json)

ee/app/services/status_page/unpublish_details_service.rb

+# frozen_string_literal: true
+
+module StatusPage
+  # Unpublish incident details from CDN.
+  #
+  # Example: An issue becomes confidential so it must be removed from CDN.
+  #
+  # This is an internal service which is part of
+  # +StatusPage::PublishIncidentService+ and is not meant to be called directly.
+  #
+  # Consider calling +StatusPage::PublishIncidentService+ instead.
+  class UnpublishDetailsService < PublishBaseService
+    private
+
+    def process(issue)
+      key = object_key(issue)
+
+      delete(key)
+    end
+
+    def object_key(issue)
+      StatusPage::Storage.details_path(issue.iid)
+    end
+  end
+end

ee/lib/status_page/storage/s3_client.rb

@@ -25,6 +25,17 @@ module StatusPage
         true
       end
 
+      # Deletes +key+ from storage
+      #
+      # Note, this operation succeeds even if +key+ does not exist in storage.
+      def delete_object(key)
+        wrap_errors(key: key) do
+          client.delete_object(bucket: bucket_name, key: key)
+        end
+
+        true
+      end
+
       private
 
       attr_reader :client, :bucket_name

ee/spec/finders/status_page/incidents_finder_spec.rb

@@ -28,7 +28,7 @@ describe StatusPage::IncidentsFinder do
     context 'for confidential issue' do
       let(:issue) { issues.fetch(:confidential) }
 
-      it { is_expected.to be_nil }
+      it { is_expected.to eq(issue) }
     end
 
     context 'for unrelated issue' do
@@ -40,6 +40,7 @@ describe StatusPage::IncidentsFinder do
 
   describe '#all' do
     let(:sorted_issues) { public_issues.sort_by(&:created_at).reverse }
+    let(:limit) { public_issues.size }
 
     subject { finder.all }
 
@@ -58,5 +59,13 @@ describe StatusPage::IncidentsFinder do
 
       it { is_expected.to eq(sorted_issues.first(1)) }
     end
+
+    context 'when combined with other finder methods' do
+      before do
+        finder.find_by_id(public_issues.first.id)
+      end
+
+      it { is_expected.to eq(sorted_issues) }
+    end
   end
 end

ee/spec/lib/status_page/storage/s3_client_spec.rb

@@ -41,6 +41,29 @@ describe StatusPage::Storage::S3Client, :aws_s3 do
     end
   end
 
+  describe '#delete_object' do
+    let(:key) { 'key' }
+
+    subject(:result) { client.delete_object(key) }
+
+    it 'returns true' do
+      stub_responses(:delete_object)
+
+      expect(result).to eq(true)
+    end
+
+    context 'when failed' do
+      let(:aws_error) { 'SomeError' }
+
+      it 'raises an error' do
+        stub_responses(:delete_object, aws_error)
+
+        msg = error_message(aws_error, key: key)
+        expect { result }.to raise_error(StatusPage::Storage::Error, msg)
+      end
+    end
+  end
+
   private
 
   def stub_responses(*args)

ee/spec/services/status_page/publish_incident_service_spec.rb

@@ -23,8 +23,9 @@ describe StatusPage::PublishIncidentService do
         .and_return(user_can_publish)
     end
 
-    context 'when publishing succeeds' do
-      it 'returns uploads incidents details and list' do
+    describe 'publish details' do
+      context 'when upload succeeds' do
+        it 'uploads incident details and list' do
           expect_to_upload_details(issue)
           expect_to_upload_list
 
@@ -32,15 +33,38 @@ describe StatusPage::PublishIncidentService do
         end
       end
 
-    context 'when uploading details fails' do
+      context 'when upload fails' do
         it 'propagates the exception' do
           expect_to_upload_details(issue, status: 404)
 
           expect { result }.to raise_error(StatusPage::Storage::Error)
         end
       end
+    end
+
+    describe 'unpublish details' do
+      let_it_be(:issue) { create(:issue, :confidential, project: project) }
+
+      context 'when deletion succeeds' do
+        it 'deletes incident details and upload list' do
+          expect_to_delete_details(issue)
+          expect_to_upload_list
+
+          expect(result).to be_success
+        end
+      end
+
+      context 'when deletion fails' do
+        it 'propagates the exception' do
+          expect_to_delete_details(issue, status: 403)
 
-    context 'when uploading list fails' do
+          expect { result }.to raise_error(StatusPage::Storage::Error)
+        end
+      end
+    end
+
+    describe 'publish list' do
+      context 'when upload fails' do
         it 'returns error and skip list upload' do
           expect_to_upload_details(issue)
           expect_to_upload_list(status: 404)
@@ -48,6 +72,7 @@ describe StatusPage::PublishIncidentService do
           expect { result }.to raise_error(StatusPage::Storage::Error)
         end
       end
+    end
 
     context 'with unrelated issue' do
       let(:issue) { create(:issue) }
@@ -71,14 +96,18 @@ describe StatusPage::PublishIncidentService do
   private
 
   def expect_to_upload_details(issue, **kwargs)
-    stub_upload_request(StatusPage::Storage.details_path(issue.iid), **kwargs)
+    stub_aws_request(:put, StatusPage::Storage.details_path(issue.iid), **kwargs)
+  end
+
+  def expect_to_delete_details(issue, **kwargs)
+    stub_aws_request(:delete, StatusPage::Storage.details_path(issue.iid), **kwargs)
   end
 
   def expect_to_upload_list(**kwargs)
-    stub_upload_request(StatusPage::Storage.list_path, **kwargs)
+    stub_aws_request(:put, StatusPage::Storage.list_path, **kwargs)
   end
 
-  def stub_upload_request(path, status: 200)
-    stub_request(:put, %r{amazonaws.com/#{path}}).to_return(status: status)
+  def stub_aws_request(method, path, status: 200)
+    stub_request(method, %r{amazonaws.com/#{path}}).to_return(status: status)
   end
 end

ee/spec/services/status_page/unpublish_details_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe StatusPage::UnpublishDetailsService do
+  let_it_be(:project, refind: true) { create(:project) }
+  let(:issue) { instance_double(Issue, iid: incident_id) }
+  let(:incident_id) { 1 }
+  let(:key) { StatusPage::Storage.details_path(incident_id) }
+
+  let(:service) { described_class.new(project: project) }
+
+  subject(:result) { service.execute(issue) }
+
+  describe '#execute' do
+    let(:status_page_setting_enabled) { true }
+    let(:storage_client) { instance_double(StatusPage::Storage::S3Client) }
+
+    let(:status_page_setting) do
+      instance_double(StatusPageSetting, enabled?: status_page_setting_enabled,
+                      storage_client: storage_client)
+    end
+
+    before do
+      stub_licensed_features(status_page: true)
+
+      allow(project).to receive(:status_page_setting)
+        .and_return(status_page_setting)
+    end
+
+    context 'when deletion succeeds' do
+      before do
+        allow(storage_client).to receive(:delete_object).with(key)
+      end
+
+      it 'removes details from CDN' do
+        expect(result).to be_success
+        expect(result.payload).to eq(object_key: key)
+      end
+    end
+
+    context 'when upload fails due to exception' do
+      let(:bucket) { 'bucket_name' }
+      let(:error) { StandardError.new }
+
+      let(:exception) do
+        StatusPage::Storage::Error.new(bucket: bucket, error: error)
+      end
+
+      before do
+        allow(storage_client).to receive(:delete_object).with(key)
+          .and_raise(exception)
+      end
+
+      it 'propagates the exception' do
+        expect { result }.to raise_error(exception)
+      end
+    end
+
+    context 'when status page setting is not enabled' do
+      let(:status_page_setting_enabled) { false }
+
+      it 'returns feature not available error' do
+        expect(result).to be_error
+        expect(result.message).to eq('Feature not available')
+      end
+    end
+  end
+end