Redirect users to pod logs for errors

To reduce exposure due to any potential SSRF, do not show or record raw
errors. This is similar to what we did in
CheckInstallationProgressService

2 files changed, 36 insertions, 26 deletions

diff --git a/app/services/clusters/applications/check_upgrade_progress_service.rb b/app/services/clusters/applications/check_upgrade_progress_service.rb
index 8ac9d82190d..11c078d15b7 100644
--- a/app/services/clusters/applications/check_upgrade_progress_service.rb
+++ b/app/services/clusters/applications/check_upgrade_progress_service.rb
@@ -14,8 +14,9 @@ module Clusters
         else
           check_timeout
         end
-      rescue ::Kubeclient::HttpError => e
-        app.make_update_errored!("Kubernetes error: #{e.message}") unless app.update_errored?
+      rescue Kubeclient::HttpError => e
+        log_error(e)
+        app.make_update_errored!("Kubernetes error: #{e.error_code}") unless app.update_errored?
       end
 
       private
@@ -27,18 +28,12 @@ module Clusters
       end
 
       def on_failed
-        app.make_update_errored!(errors || 'Update silently failed')
-      ensure
-        remove_pod
+        app.make_update_errored!("Update failed. Check pod logs for #{upgrade_command.pod_name} for more details.")
       end
 
       def check_timeout
         if timeouted?
-          begin
-            app.make_update_errored!('Update timed out')
-          ensure
-            remove_pod
-          end
+          app.make_update_errored!("Update timed out. Check pod logs for #{upgrade_command.pod_name} for more details.")
         else
           ::ClusterWaitForAppUpdateWorker.perform_in(
             ::ClusterWaitForAppUpdateWorker::INTERVAL, app.name, app.id)
diff --git a/spec/services/clusters/applications/check_upgrade_progress_service_spec.rb b/spec/services/clusters/applications/check_upgrade_progress_service_spec.rb
index 045e490b8e4..939c9b79a94 100644
--- a/spec/services/clusters/applications/check_upgrade_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_upgrade_progress_service_spec.rb
@@ -4,21 +4,13 @@ require 'spec_helper'
 
 describe Clusters::Applications::CheckUpgradeProgressService do
   RESCHEDULE_PHASES = ::Gitlab::Kubernetes::Pod::PHASES -
-    [::Gitlab::Kubernetes::Pod::SUCCEEDED, ::Gitlab::Kubernetes::Pod::FAILED, ::Gitlab].freeze
+    [::Gitlab::Kubernetes::Pod::SUCCEEDED, ::Gitlab::Kubernetes::Pod::FAILED].freeze
 
   let(:application) { create(:clusters_applications_prometheus, :updating) }
   let(:service) { described_class.new(application) }
   let(:phase) { ::Gitlab::Kubernetes::Pod::UNKNOWN }
   let(:errors) { nil }
 
-  shared_examples 'a terminated upgrade' do
-    it 'removes the POD' do
-      expect(service).to receive(:remove_pod).once
-
-      service.execute
-    end
-  end
-
   shared_examples 'a not yet terminated upgrade' do |a_phase|
     let(:phase) { a_phase }
 
@@ -38,15 +30,13 @@ describe Clusters::Applications::CheckUpgradeProgressService do
       context 'when timed out' do
         let(:application) { create(:clusters_applications_prometheus, :timeouted, :updating) }
 
-        it_behaves_like 'a terminated upgrade'
-
         it 'make the application update errored' do
           expect(::ClusterWaitForAppUpdateWorker).not_to receive(:perform_in)
 
           service.execute
 
           expect(application).to be_update_errored
-          expect(application.status_reason).to eq("Update timed out")
+          expect(application.status_reason).to eq("Update timed out. Check pod logs for upgrade-prometheus for more details.")
         end
       end
     end
@@ -63,7 +53,11 @@ describe Clusters::Applications::CheckUpgradeProgressService do
     context 'when upgrade pod succeeded' do
       let(:phase) { ::Gitlab::Kubernetes::Pod::SUCCEEDED }
 
-      it_behaves_like 'a terminated upgrade'
+      it 'removes the upgrade pod' do
+        expect(service).to receive(:remove_pod).once
+
+        service.execute
+      end
 
       it 'make the application upgraded' do
         expect(::ClusterWaitForAppUpdateWorker).not_to receive(:perform_in)
@@ -79,16 +73,37 @@ describe Clusters::Applications::CheckUpgradeProgressService do
       let(:phase) { ::Gitlab::Kubernetes::Pod::FAILED }
       let(:errors) { 'test installation failed' }
 
-      it_behaves_like 'a terminated upgrade'
-
       it 'make the application update errored' do
         service.execute
 
         expect(application).to be_update_errored
-        expect(application.status_reason).to eq(errors)
+        expect(application.status_reason).to eq("Update failed. Check pod logs for upgrade-prometheus for more details.")
       end
     end
 
     RESCHEDULE_PHASES.each { |phase| it_behaves_like 'a not yet terminated upgrade', phase }
+
+    context 'when upgrade raises a Kubeclient::HttpError' do
+      let(:cluster) { create(:cluster, :provided_by_user, :project) }
+
+      before do
+        application.update!(cluster: cluster)
+
+        expect(service).to receive(:phase).and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil))
+      end
+
+      it 'shows the response code from the error' do
+        service.execute
+
+        expect(application).to be_update_errored
+        expect(application.status_reason).to eq('Kubernetes error: 401')
+      end
+
+      it 'should log error' do
+        expect(service.send(:logger)).to receive(:error)
+
+        service.execute
+      end
+    end
   end
 end